-
Notifications
You must be signed in to change notification settings - Fork 430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvement: Adds field PeriodicTask.origin_key to sync tasks defined in source code with settings.beat_schedule #136
base: main
Are you sure you want to change the base?
Conversation
…asks that are deleted from settings.beat_schedule in source code Bugfix: changing schedule class in settings.CELERYBEAT_SCHEDULE will no longer create a second (or third) schedule in PeriodicTask instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new pr has merged upstream, pulling that and regenerating the
migrations by deleting the one here and a test case to verify the change is needed.
} | ||
schedules[model_field] = model_schedule | ||
|
||
entry.update(**schedules) | ||
entry.update( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this will run two queries instead of one now? It would be nicer to keep it as one.
) | ||
existing_tasks = set( | ||
map(lambda x: x.name, existing_task_instances) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could replace a lot of logic here with just this I think:
tasks_to_purge = PeriodicTask.objects.filter(
origin_key=origin_key).exclude(name__in=mapping.keys())
if tasks_to_purge:
tasks_to_purge.delete()
log.warn(...)
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('django_celery_beat', '0006_auto_20180210_1226'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other migrations has been added, you'll need to rebase this to be the next one in the list based on what's in the master branch.
This adds a field
PeriodicTask.origin_key
which indicates if a task database row was populated dynamically or from source code (e.g.settings.CELERYBEAT_SCHEDULE
orsettings.beat_schedule
in4.1+
).If an entry is removed from
settings.CELERYBEAT_SCHEDULE
it will also delete any previously populated database row forPeriodicTask
.E.g.
After commenting out the above, celery beat would purge the task from database and also emit this logger statement:
I also updated the scheduler to only ingest one and only one schedule when reading
settings.CELERYBEAT_SCHEDULE
. The old behavior was that if you re-wrote an interval schedule to a cron schedule, it would remain associated with the old (now deleted) interval schedule and be called for both schedules. E.g.: